From 69caa63b3d23d8f5412697edd0ecdd1ca025ed96 Mon Sep 17 00:00:00 2001 From: Nick Cameron Date: Wed, 21 Dec 2016 17:34:05 +1300 Subject: [PATCH] Review changes --- src/bin/check.rs | 58 ++++++++++++++++++-- src/cargo/ops/cargo_check.rs | 90 +++----------------------------- src/cargo/ops/cargo_compile.rs | 23 ++++++++ src/cargo/ops/cargo_rustc/mod.rs | 42 +++++++++------ src/cargo/ops/mod.rs | 3 +- 5 files changed, 110 insertions(+), 106 deletions(-) diff --git a/src/bin/check.rs b/src/bin/check.rs index 8ea6747cd..61fd7a33a 100644 --- a/src/bin/check.rs +++ b/src/bin/check.rs @@ -1,10 +1,9 @@ use std::env; -use cargo::ops::{self}; -use cargo::ops::cargo_check::{Options, with_check_env}; +use cargo::ops::{self, CompileOptions, MessageFormat}; +use cargo::ops::with_check_ws; use cargo::util::{CliResult, Config}; - pub const USAGE: &'static str = " Check a local package and all of its dependencies for errors @@ -43,12 +42,61 @@ the manifest. The default profile for this command is `dev`, but passing the --release flag will use the `release` profile instead. "; +#[derive(RustcDecodable)] +pub struct Options { + flag_package: Vec, + flag_jobs: Option, + flag_features: Vec, + flag_all_features: bool, + flag_no_default_features: bool, + flag_target: Option, + flag_manifest_path: Option, + flag_verbose: u32, + flag_quiet: Option, + flag_color: Option, + flag_message_format: MessageFormat, + flag_release: bool, + flag_lib: bool, + flag_bin: Vec, + flag_example: Vec, + flag_test: Vec, + flag_bench: Vec, + flag_locked: bool, + flag_frozen: bool, +} + pub fn execute(options: Options, config: &Config) -> CliResult> { debug!("executing; cmd=cargo-check; args={:?}", env::args().collect::>()); - with_check_env(options, config, |ws, opts| { - ops::compile(ws, opts)?; + config.configure(options.flag_verbose, + options.flag_quiet, + &options.flag_color, + options.flag_frozen, + options.flag_locked)?; + + with_check_ws(options.flag_manifest_path.clone(), config, |ws| { + let opts = CompileOptions { + config: config, + jobs: options.flag_jobs, + target: options.flag_target.as_ref().map(|t| &t[..]), + features: &options.flag_features, + all_features: options.flag_all_features, + no_default_features: options.flag_no_default_features, + spec: ops::Packages::Packages(&options.flag_package), + mode: ops::CompileMode::Check, + release: options.flag_release, + filter: ops::CompileFilter::new(options.flag_lib, + &options.flag_bin, + &options.flag_test, + &options.flag_example, + &options.flag_bench), + message_format: options.flag_message_format, + target_rustdoc_args: None, + target_rustc_args: None, + }; + + ops::compile(ws, &opts)?; Ok(None) }) } diff --git a/src/cargo/ops/cargo_check.rs b/src/cargo/ops/cargo_check.rs index b8893efa9..2e78a9c0f 100644 --- a/src/cargo/ops/cargo_check.rs +++ b/src/cargo/ops/cargo_check.rs @@ -1,89 +1,13 @@ use core::Workspace; -use ops::{self, CompileOptions, MessageFormat}; -use util::important_paths::{find_root_manifest_for_wd}; +use util::important_paths::find_root_manifest_for_wd; use util::{CliResult, Config}; -#[derive(RustcDecodable)] -pub struct Options { - flag_package: Vec, - flag_jobs: Option, - flag_features: Vec, - flag_all_features: bool, - flag_no_default_features: bool, - flag_target: Option, - flag_manifest_path: Option, - flag_verbose: u32, - flag_quiet: Option, - flag_color: Option, - flag_message_format: MessageFormat, - flag_release: bool, - flag_lib: bool, - flag_bin: Vec, - flag_example: Vec, - flag_test: Vec, - flag_bench: Vec, - flag_locked: bool, - flag_frozen: bool, -} - -impl Options { - pub fn default() -> Options { - Options { - flag_package: vec![], - flag_jobs: None, - flag_features: vec![], - flag_all_features: false, - flag_no_default_features: false, - flag_target: None, - flag_manifest_path: None, - flag_verbose: 0, - flag_quiet: None, - flag_color: None, - flag_message_format: MessageFormat::Human, - flag_release: false, - flag_lib: false, - flag_bin: vec![], - flag_example: vec![], - flag_test: vec![], - flag_bench: vec![], - flag_locked: false, - flag_frozen: false, - } - } -} - - -pub fn with_check_env(options: Options, config: &Config, f: F) -> CliResult> - where F: FnOnce(&Workspace, &CompileOptions) -> CliResult> +pub fn with_check_ws(flag_manifest_path: Option, + config: &Config, f: F) + -> CliResult> + where F: FnOnce(&Workspace) -> CliResult> { - config.configure(options.flag_verbose, - options.flag_quiet, - &options.flag_color, - options.flag_frozen, - options.flag_locked)?; - - let root = find_root_manifest_for_wd(options.flag_manifest_path, config.cwd())?; - - let opts = CompileOptions { - config: config, - jobs: options.flag_jobs, - target: options.flag_target.as_ref().map(|t| &t[..]), - features: &options.flag_features, - all_features: options.flag_all_features, - no_default_features: options.flag_no_default_features, - spec: ops::Packages::Packages(&options.flag_package), - mode: ops::CompileMode::Check, - release: options.flag_release, - filter: ops::CompileFilter::new(options.flag_lib, - &options.flag_bin, - &options.flag_test, - &options.flag_example, - &options.flag_bench), - message_format: options.flag_message_format, - target_rustdoc_args: None, - target_rustc_args: None, - }; - + let root = find_root_manifest_for_wd(flag_manifest_path, config.cwd())?; let ws = Workspace::new(&root, config)?; - f(&ws, &opts) + f(&ws) } diff --git a/src/cargo/ops/cargo_compile.rs b/src/cargo/ops/cargo_compile.rs index cce8fc2b6..38e078259 100644 --- a/src/cargo/ops/cargo_compile.rs +++ b/src/cargo/ops/cargo_compile.rs @@ -62,6 +62,29 @@ pub struct CompileOptions<'a> { pub target_rustc_args: Option<&'a [String]>, } +impl<'a> CompileOptions<'a> { + pub fn with_default(config: &Config, mode: CompileMode, f: F) -> T + where F: FnOnce(CompileOptions) -> T + { + let opts = CompileOptions { + config: config, + jobs: None, + target: None, + features: &[], + all_features: false, + no_default_features: false, + spec: ops::Packages::Packages(&[]), + mode: mode, + release: false, + filter: ops::CompileFilter::new(false, &[], &[], &[], &[]), + message_format: MessageFormat::Human, + target_rustdoc_args: None, + target_rustc_args: None, + }; + f(opts) + } +} + #[derive(Clone, Copy, PartialEq, Debug)] pub enum CompileMode { Test, diff --git a/src/cargo/ops/cargo_rustc/mod.rs b/src/cargo/ops/cargo_rustc/mod.rs index 23107efab..158e72aa2 100644 --- a/src/cargo/ops/cargo_rustc/mod.rs +++ b/src/cargo/ops/cargo_rustc/mod.rs @@ -59,10 +59,26 @@ pub type PackagesToBuild<'a> = [(&'a Package, Vec<(&'a Target, &'a Profile)>)]; /// directly, we'll use an Executor, giving clients an opportunity to intercept /// the build calls. pub trait Executor: Clone + Send + 'static { - fn init(&mut self, cx: &Context); + fn init(&mut self, _cx: &Context) {} /// If execution succeeds, the ContinueBuild value indicates whether Cargo /// should continue with the build process for this package. - fn exec(&self, cmd: ProcessBuilder, id: &PackageId) -> Result; + fn exec(&self, cmd: ProcessBuilder, _id: &PackageId) -> Result { + cmd.exec()?; + Ok(ContinueBuild::Continue) + } + + fn exec_json(&self, + cmd: ProcessBuilder, + _id: &PackageId, + mut handle_stdout: F1, + mut handle_srderr: F2) + -> Result + where F1: FnMut(&str) -> CargoResult<()>, + F2: FnMut(&str) -> CargoResult<()>, + { + cmd.exec_with_streaming(&mut handle_stdout, &mut handle_srderr)?; + Ok(ContinueBuild::Continue) + } } /// A DefaultExecutorcalls rustc without doing anything else. It is Cargo's @@ -70,14 +86,7 @@ pub trait Executor: Clone + Send + 'static { #[derive(Copy, Clone)] pub struct DefaultExecutor; -impl Executor for DefaultExecutor { - fn init(&mut self, _cx: &Context) {} - - fn exec(&self, cmd: ProcessBuilder, _id: &PackageId) -> Result { - cmd.exec()?; - Ok(ContinueBuild::Continue) - } -} +impl Executor for DefaultExecutor {} #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub enum ContinueBuild { @@ -327,13 +336,13 @@ fn rustc(cx: &mut Context, unit: &Unit, exec: &mut E) -> CargoResul state.running(&rustc); let cont = if json_messages { - rustc.exec_with_streaming( - &mut |line| if !line.is_empty() { + exec.exec_json(rustc, &package_id, + |line| if !line.is_empty() { Err(internal(&format!("compiler stdout is not empty: `{}`", line))) } else { Ok(()) }, - &mut |line| { + |line| { // stderr from rustc can have a mix of JSON and non-JSON output if line.starts_with("{") { // Handle JSON lines @@ -351,11 +360,10 @@ fn rustc(cx: &mut Context, unit: &Unit, exec: &mut E) -> CargoResul writeln!(io::stderr(), "{}", line)?; } Ok(()) - }, - ).map(|_| ()).chain_error(|| { + } + ).chain_error(|| { human(format!("Could not compile `{}`.", name)) - })?; - ContinueBuild::Continue + })? } else { exec.exec(rustc, &package_id).chain_error(|| { human(format!("Could not compile `{}`.", name)) diff --git a/src/cargo/ops/mod.rs b/src/cargo/ops/mod.rs index 0b59fb923..4f3751890 100644 --- a/src/cargo/ops/mod.rs +++ b/src/cargo/ops/mod.rs @@ -1,3 +1,4 @@ +pub use self::cargo_check::with_check_ws; pub use self::cargo_clean::{clean, CleanOptions}; pub use self::cargo_compile::{compile, compile_with_exec, compile_ws, CompileOptions}; pub use self::cargo_compile::{CompileFilter, CompileMode, MessageFormat, Packages}; @@ -24,7 +25,7 @@ pub use self::cargo_pkgid::pkgid; pub use self::resolve::{resolve_ws, resolve_ws_precisely, resolve_with_previous}; pub use self::cargo_output_metadata::{output_metadata, OutputMetadataOptions, ExportInfo}; -pub mod cargo_check; +mod cargo_check; mod cargo_clean; mod cargo_compile; mod cargo_doc; -- 2.30.2